-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support glob patterns #151
Conversation
We use a weird mix of multiple glob patterns and ignores which means we parse the available files in an inefficient way. Instead just use a single glob pattern which can do everything we need. Fixes ipfs/js-ipfs#2885
src/files/glob-source.js
Outdated
const absolutePath = Path.resolve(process.cwd(), path) | ||
const stat = await fsp.stat(absolutePath) | ||
const prefix = Path.dirname(absolutePath) | ||
const globOptions = Object.assign({}, pattern, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pattern is a string no? Why assigning options to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent point.
src/files/glob-source.js
Outdated
) | ||
} | ||
if (pattern.startsWith('.' + Path.sep)) { | ||
pattern = pattern.replace('.' + Path.sep, '') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this bit guard against?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With hindsight I don't think it's necessary, I've removed it.
src/files/glob-source.js
Outdated
* @param {boolean} [options.followSymlinks] - follow symlinks | ||
* @param {boolean} [options.preserveMode] - preserve mode | ||
* @param {boolean} [options.preserveMtime] - preserve mtime | ||
* @param {number} [options.mode] - mode to use - if preserveMode is true this will be ignored | ||
* @param {import('ipfs-unixfs').MtimeLike} [options.mtime] - mtime to use - if preserveMtime is true this will be ignored | ||
* @yields {Object} File objects in the form `{ path: String, content: AsyncIterator<Buffer> }` | ||
*/ | ||
module.exports = async function * globSource (paths, options) { | ||
module.exports = async function * globSource (pattern, options) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be tempted to make this take cwd, pattern, options
to avoid some of the cwd messing around bits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, I'm, always wondering where the output path will start.
We use a weird mix of multiple glob patterns and ignores which means we parse the available files in an inefficient way.
Instead just use a single glob pattern which can do everything we need.
Fixes ipfs/js-ipfs#2885
BREAKING CHANGE: the globSource call signature has changed and no longer supports the recursive or ignore options